dash: wire the embedded arm so --run can actually take it (#738) - #891
Conversation
92a0d27 to
0ca0de0
Compare
The daemonless embedded template stack (SML/quorum CCbTx, DKG-window
serving, BLS-verified commitments, superblock/credit-pool/payee guards)
is merged and soak-proven, but no released --run invocation could reach
it. Taking the embedded arm needs TWO independent conditions:
(1) the work-source arm gate -- DASHWorkSource only consults the
embedded bundle when `is_testnet_ || embedded_mainnet_`
(work_source.cpp get_work / resource_template_now);
(2) a live coin-state FEED -- NodeCoinState::populated() only flips
when CoinStateMaintainer publishes a tip, and the maintainer +
header chain + 6 ingest legs are constructed ONLY inside
main_dash's `if (coin_p2p)` block.
No single flag satisfied both. --embedded-mainnet armed (1) only, so
the bundle was never fed and populated() stayed false forever;
--coin-p2p-connect/--coin-p2p-discover armed (2) only. On top of that
--embedded-mainnet was absent from --help entirely, so the undocumented
combination was undiscoverable. Net effect: 100% of released --run
templates came off the dashd fallback and the whole daemonless stack
was unreachable scaffolding.
resolve_embedded_arm() (arm_resolution.hpp) makes the decision once, as
one pure function, with a strictly ONE-WAY implication:
embedded opt-in => coin-state feed (new -- this is the wiring)
coin-state feed => embedded opt-in (never -- explicitly not)
So --embedded-mainnet now arms both halves end to end (seed-based
discovery when no peer is pinned), while a transport flag still cannot
move the arm.
REWARD SAFETY (this node class runs a live production mining hotel, and
there is a documented incident where --coin-p2p-connect activated an
unguarded embedded arm on a live node):
* a default --run resolves arm=dashd-fallback, unchanged;
* --coin-p2p-connect alone resolves arm=dashd-fallback, unchanged --
it brings up the transport and nothing else;
* both are pinned by tests, including an exhaustive sweep of the
un-opted-in mainnet argv quadrant;
* verified empirically: normalized run-log diff of the pre-change vs
post-change binary for both invocations differs ONLY by the two
added diagnostic lines.
Also: --embedded-mainnet documented in --help, and a startup line that
names the resolved arm plus the reason, so the arm is field-checkable
instead of inferred.
Tests fold into the existing allowlisted test_dash_stratum_work_source
target (no new add_executable -> no NOT_BUILT sentinel); the allowlist
drift-guard passes.
The E1 block comment still said coin_p2p is null without --coin-p2p-connect; --embedded-mainnet now implies a feed. Restate the no-op guarantee over the resolved feed condition and spell out that arming the feed alone still cannot move the arm. Comment-only.
The DASH coin peer manager exposed set_http_peer_seeds (coin_peer_manager.hpp:303) as the third bootstrap fallback tier (after DNS + fixed seeds) but main_dash.cpp never called it, leaving that tier dark. LTC wires it at main_ltc.cpp:1875. Add the equivalent call in the coin_p2p_discover arm so the daemonless embedded path has the same three-tier peer bootstrap.
0ca0de0 to
a41e9c2
Compare
The Windows red is NOT clearing, and it is #891-specificRe-ran and re-measured. Correcting the earlier "secp256k1 git-clone flake" read — it survived a re-run, which a clone flake would not. Isolated to this PR. The same Windows check passes right now on: Three other open PRs, same runners, same step, green. So this is not a runner or dependency-fetch problem — the asymmetry points at #891. Why I am not calling it, and what I need from youI have mis-diagnosed CI failures in both directions this week and would rather hand you evidence than a guess:
So: Please pull the raw step log. Two things worth checking first:
Why this matters more than a normal red#891 is the contabo blocker, and now with live evidence behind it. A 30 h daemonless soak measured 0 coin-network peers against 4 sharechain peers — same process, same host. The coin dialer never runs, because Contabo has since been rebuilt to Acceptance signal is concrete: the coin-network cards on |
|
1 participant
Draft. Refs #738.
The exact reason the embedded arm was never taken
Taking the daemonless embedded template arm requires two independent conditions, and before this PR no single flag satisfied both:
DASHWorkSourceonly consults the embedded bundle whenis_testnet_ || embedded_mainnet_(src/impl/dash/stratum/work_source.cpp:327, guard at:205). On mainnet that means--embedded-mainnet.NodeCoinState::populated()only flips onceCoinStateMaintainerpublishes a tip, and the maintainer + header chain + 5 ingest legs are constructed only insidemain_dash.cpp'sif (coin_p2p)block (src/c2pool/main_dash.cpp:2164).coin_p2pitself was built only underif (!coin_p2p_targets.empty() || coin_p2p_discover).--embedded-mainnetarmed (1) only — the bundle was never fed,populated()stayed false forever, and every template came off the dashd fallback.--coin-p2p-connect/--coin-p2p-discoverarmed (2) only. On top of that--embedded-mainnetwas absent from--helpentirely, so even the undocumented two-flag combination was undiscoverable.Net effect: 100% of released
--runtemplates came off the dashd fallback, and the entire merged, soak-proven daemonless stack (#826: SML/quorum CCbTx, DKG-window serving, BLS-verified commitments, superblock/credit-pool/payee guards) was unreachable scaffolding.What was wired
resolve_embedded_arm()(newsrc/impl/dash/coin/arm_resolution.hpp) makes the decision once, as one pure, dependency-free function, with a strictly one-way implication:--embedded-mainnetnow arms both halves end to end (seed-based discovery when no peer is pinned; an operator-named transport is never overridden). A transport flag still cannot move the arm.main_dash.cppis the sole production caller; the fourtestnet || embedded_mainnetsites and the coin-P2P construction predicate now read from the single resolution.--embedded-mainnetis documented in--help, and a startup line names the resolved arm plus the reason, so the arm is field-checkable instead of inferred.Reward safety — how default-arm behaviour was proven unchanged
This node class runs a live production mining hotel, and there is a documented incident where
--coin-p2p-connectactivated an unguarded embedded arm on a live node.Proof by construction. For a default run (
testnet=false, embedded_mainnet=false, no coin-P2P flags) every changed predicate evaluates to the identical value:!coin_p2p_targets.empty() || coin_p2p_discoverrun_arm.coin_feed_armedfalse->falsetestnet || embedded_mainnetrun_arm.embedded_arm_enabledfalsecoin_p2p_discovercoin_p2p_discover_efffalse->falsework_source->set_embedded_mainnet(embedded_mainnet)still reads the raw flag, not the resolution — defence in depth: even a wrong resolver cannot lift the work-source gate.Proof by A/B binary diff. Pre-change and post-change
c2pool-dashbinaries were built from the same tree and run on both non-embedded invocations. Coin-network subsystem activity is identical:EMB-DASH/COIN-P2Plines--run--coin-p2p-connectaloneNormalized control-flow diff shows only the help text and the two new diagnostic lines; the residual is live-sharechain P2P timing noise of the same class a PRE-vs-PRE self-diff control also produces.
Observed at runtime:
Reachability proof (not just a passing test)
Per the #873 / #878 / #881 lesson — passing tests wired into unreachable code — the wiring was proven reachable in the real binary against live mainnet, not only in a test mirror:
That
new_tip fired (maintainer arm)line is the exact leg that flipspopulated()— previously dead in every released binary.Caller-side lock trace: the wiring adds no call into a self-locking method.
resolve_embedded_arm()is pure (no I/O, no allocation, no locks) and is called once inrun_nodebeforeioc.run(), i.e. single-threaded startup.NodeCoinStateholds no mutex at all — it is io_context-thread confined, with maintainer writes andget_work()reads on the same thread — so the "exclusive lock held across a self-locking callee" defect class does not arise. The only changed values are startup-time predicates.Tests
Folded into the existing allowlisted
test_dash_stratum_work_sourcetarget (.github/workflows/build.yml:147and:335) — no newadd_executable, per #868.New
DashRunArmResolutionsuite, 8 tests. Each goes beyond restating the resolver:served_arm_for()composes the resolution exactly asrun_nodedoes and asks the realDASHWorkSource::get_work()which arm it serves.DefaultRunResolvesDashdFallback— reward-safety pin 1CoinP2pConnectAloneStillResolvesDashdFallback— reward-safety pin 2 (the hotel incident)CoinP2pDiscoverAloneStillResolvesDashdFallbackEmbeddedMainnetOptInArmsBothHalvesEndToEnd— the wiringEmbeddedMainnetWithPinnedPeerDoesNotImplyDiscoveryTestnetWithoutFeedIsUnchangedFallback/TestnetWithCoinP2pIsUnchangedEmbedded— no regressionNoMainnetCombinationWithoutOptInEverEnablesTheArm— exhaustive sweep of the un-opted-in mainnet argv quadrantResult: 52/52 pass in the full target.
Negative controls (both confirm the pins actually bite):
embedded_arm_enabled |= coin_p2p_connect || coin_p2p_discover, i.e. reintroducing the hotel-incident bug) -> reds exactly the 3 safety pins.discover_implied = false, pre-dash: embedded arm unwired in --run — NodeCoinState never seeds (v0.2.3 differential: FALLBACK-ONLY) #738 behaviour) -> redsEmbeddedMainnetOptInArmsBothHalvesEndToEnd.Scope / what this does NOT do
This closes item 1 of #738's four-item EMBEDDED-READY list (wire the feed into the work source). Items 2-4 remain open: BIP35 connect-time mempool pull, the
fee_knownpricing ladder, and the live differential re-run as acceptance gate. Every per-template viability gate (SML fresh at tip, non-superblock, credit-pool seed height, bestCL, MN-payee cursor, DKG plan) still fails closed to the dashd fallback — arming the arm is not the same as serving from it. Observed in the live run:E2c MN-set seed UNAVAILABLE ... templates keep routing to the fallback arm.Related, NOT fixed here (owned by a sibling agent)
C2POOL_DASH_BLS=ONis set by no CI or release workflow (grep -rn C2POOL_DASH_BLS .github/workflows/-> zero hits; the option defaultsOFFatCMakeLists.txt:122). Shipped binaries are therefore BLS-dark and Phase-L can never serve a real commitment in the field.Trap for the record: the flag ON with the lib ABSENT is a
message(WARNING ...), notFATAL_ERROR(CMakeLists.txt:154) — it setsC2POOL_DASH_BLS_AVAILABLE=OFF, compiles the fail-closed null-serve stub, and the build still succeeds. A CI job that flips the flag without also providing dashbls would look green while shipping the stub.